fix(weather): resolve GFS precip twin crash + expose cloud_cover_pct, visibility_m, cloud_ceiling_m (#63)#68
Merged
helloiamvu merged 3 commits intoJun 6, 2026
Conversation
added 3 commits
June 5, 2026 09:35
…stlyrightmd#63) - Resolve the latent GFS precipitation twin bug by adding a `_pick_record` helper to forecast_nwp.py. It filters duplicates by prioritizing instantaneous records and breaking ties using lowest `record_no`. - Implement `cloud_cover_pct`, `visibility_m`, and `cloud_ceiling_m` for GFS and HRRR. - Define and integrate physics-bounds QC rules for the three new fields. - Regenerate schema.forecast_nwp.v1 JSON schemas. - Add comprehensive test coverage in test_qc_rules_nwp.py, test_forecast_nwp.py, and test_forecast_nwp_multi_cycle.py. - Include planning artifacts and research briefs in .briefs/ directory.
Reverted accidental inclusion of all extras in root dependency list. The [nwp] extra pulls eccodes which is not available in CI without the full extra install, causing test detection guards to pass while the actual eccodes binary fails to load.
Member
|
Codex review (gpt-5.5, medium reasoning) — 1 blocking finding (P2):
CI is green and the Python disambiguation logic + GFS-precip fix look solid; this is the one issue to resolve before merge. (Reviewed in the 2026-06-06 autonomous triage run — feature-scope, recommend a 1.6.0 minor.) |
5 tasks
helloiamvu
added a commit
that referenced
this pull request
Jun 6, 2026
… P2) Codex P2 on #68: scripts/export_schemas.py exported schema.forecast_nwp.v1 but the TS codegen SCHEMA_FILES list omitted it, so pnpm codegen never emitted a ForecastNwpV1 type/validator and TS consumers stayed unaware of the new public NWP columns (dual-SDK parity gap). Add the schema to the TS codegen list and regenerate: new generated forecast_nwp.v1.ts + ajv validator + barrel/format-map exports. Codegen + core validator tests green.
helloiamvu
added a commit
that referenced
this pull request
Jun 6, 2026
…-join fix (#67) + docs (#52) Minor release (features added -> 1.6.0, not a patch). Bump all three packages 1.5.3 -> 1.6.0; regenerate uv.lock (codex P2 on #71: lock was stale at 1.5.2/1.5.3). CHANGELOG: fold the never-published 1.5.3 entry into 1.6.0 and add the #63 NWP-fields and #64 OM-cache/throttle feature entries. Bundles PRs #67/#52 (already in merged-vision) + #66/#64 + #68/#63, each with its codex P2 resolved on this integration branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #63. Resolves a latent
GribIntegrityErrorcrash that affects everyforecast_nwp(station, "gfs")call at the defaultfxx=1, and adds threenew NWP forecast columns:
cloud_cover_pct,visibility_m, andcloud_ceiling_mfor both HRRR and GFS models.The latent GFS precip bug
GFS GRIB2 files contain duplicate
APCP:surfacerecords atfxx >= 1(two identical accumulation windows with different record numbers). The SDK's
ambiguity guard in
_extract_records()raisesGribIntegrityErroron anyduplicate, completely aborting the fetch. Both mirrors carry the same twins, so
mirror fallback doesn't recover. At
fxx=0the bug is masked because GFS omitsAPCP entirely at the analysis hour.
This was invisible because CI has no live GFS test — only HRRR.
Fix:
_pick_recordheuristicReplaces the fatal
GribIntegrityErrorwith a deterministic picker that:record_noThis simultaneously fixes the APCP crash and unblocks
cloud_cover_pcton GFS,where
(TCDC, entire atmosphere)also produces two records (instantaneous vstime-averaged). HRRR is unaffected — all fields are single-record.
New columns
cloud_cover_pcttccvisibility_mviscloud_ceiling_mghNaming uses
cloud_ceiling_m(notceiling_m) for cross-source consistencywith IEM adapters.
Supporting evidence
Academic and operational research in
.briefs/demonstrates why these fieldsmatter for prediction-market temperature modeling:
highs by 7–11°F and lows by 8–10°F. That's the exact margin around Kalshi
strike prices.
regression equations. The SDK was missing what MOS considers essential.
cover as a core 2D input field for image-to-image temperature correction.
enables quants to detect and correct for this known radiative bias.
See
.briefs/cloud-cover-deep-research.md(64 cited sources) and.briefs/github-issue-63-nwp-fields-review.mdfor full analysis.Changes
Bug fix
forecast_nwp.py: Add_pick_record()disambiguation helperforecast_nwp.py: Replaceraise GribIntegrityErrorwith warning + pickerNew fields
schemas/forecast_nwp.py(core): Register 3 nullable float64 columns_nwp_grids/hrrr.py,_nwp_grids/gfs.py: Add TCDC, VIS, HGT mappingsforecast_nwp.py: Add cfgrib short-name lookups (tcc, vis, gh)forecast_nwp.py: Register innullable_numeric_colsand_empty_dataframeqc/rules_nwp.py: Add physics-bounds QC rules (cloud 0–100%, vis/ceiling ≥ 0)schemas/json/schema.forecast_nwp.v1.json: RegeneratedTests
TestDisambiguationHeuristics: synthetic duplicate .idx fixtures, window vsinstantaneous preference, record_no tiebreak
test_qc_rules_nwp.py: updated base rule count (7 → 10)test_forecast_nwp_multi_cycle.py: mock row structures updatedTest plan
uv run pytest packages/weather/tests -m "not live"uv run pytest -m "not live"fix/63-nwp-cloud-cover-precipfromupstream/mainCross-SDK Sync (see .planning/CROSS-SDK-SYNC.md)
python_only: true — external contributor PR; TS parity ticket will be filed
by a maintainer after merge. The schema column additions and
.idxdisambiguation heuristic both need TS ports (documented in PR body and
.briefs/github-issue-63-nwp-fields-review.md§6).